Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[PLA-2042] fix track collection loading #167

Merged
merged 2 commits into from
Oct 21, 2024
Merged

[PLA-2042] fix track collection loading #167

merged 2 commits into from
Oct 21, 2024

Conversation

zlayine
Copy link
Contributor

@zlayine zlayine commented Oct 21, 2024 •
edited by github-actions bot
Loading

PR Type

Bug fix

Description

  • Fixed an issue with track collection loading by adding a watcher on the isOpen prop to reset the collectionId and loading state when the modal is closed.
  • Imported watch from Vue to implement the new functionality.

Changes walkthrough 📝

Relevant files
Bug fix
TrackCollectionModal.vue
Fix track collection loading by resetting state on close 

resources/js/components/TrackCollectionModal.vue

  • Added watch from Vue to monitor isOpen prop.
  • Reset collectionId and loading when modal is closed.
    		•  +11/-1   

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    @zlayine zlayine self-assigned this Oct 21, 2024
    @github-actions GitHub Actions
    Copy link

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Code Redundancy
    The reset logic for collectionId and loading is duplicated in both onMounted and watch functions. Consider creating a separate method to handle this reset logic and call this method in both places to reduce redundancy and improve maintainability.

    @github-actions GitHub Actions
    Copy link

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Performance
    Improve performance by debouncing the watcher for props.isOpen

    Consider debouncing the watcher for props.isOpen to avoid potential performance
    issues if the isOpen prop changes frequently.

    resources/js/components/TrackCollectionModal.vue [58-64]

    +import { debounce } from 'lodash';
+...
 watch(
     () => props.isOpen,
-    (isOpen) => {
+    debounce((isOpen) => {
         if (!isOpen) {
             collectionId.value = '';
             loading.value = false;
         }
-    }
+    }, 200)
 )
    Suggestion importance[1-10]: 7

    Why: Debouncing the watcher for props.isOpen can improve performance by reducing the frequency of updates when the prop changes rapidly. This is a relevant and beneficial enhancement, especially in scenarios where the isOpen prop might toggle frequently, thus preventing unnecessary reactivity and potential performance issues. However, the suggestion assumes the use of lodash, which may not be necessary if lodash is not already a dependency.

    		7

    @zlayine zlayine merged commit 2325d31 into master Oct 21, 2024
    4 checks passed
    @zlayine zlayine deleted the bugfix/pla-2042/track-loading branch October 21, 2024 11:21
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Reviewers

    @v16Studios v16Studios v16Studios approved these changes

    @leonardocustodio leonardocustodio Awaiting requested review from leonardocustodio

    @enjinabner enjinabner Awaiting requested review from enjinabner

    Assignees

    @zlayine zlayine

    Labels
    Bug fix Review effort [1-5]: 2
    Milestone
    No milestone
    Development

    Successfully merging this pull request may close these issues.
    2 participants
    @zlayine @v16Studios